Enable Qt framework in CI and refactoring of CI configuration #160
Conversation
Use the 'option' command for user-selectable options instead of cached bool variables. Just like CMake recommends. Also delete some excess whitespace.
Also use the more localized target_include_directories instead of include_directories and AUTOMOC target property instead of CMAKE_AUTOMOC (which is the default for the value of all target's AUTOMOC property).
* remove 'cmd' prefix - because this is already the default * remove intermediate RUBY_VERSION environment variable * remove unused environment variables * rename %PLATFORM% to %MSVC_ARCH% where used * distinguish platform by presence of %MINGW_ARCH% or %MSVC_ARCH%
To ensure our Qt-using code gets tested by our CI systems as well.
That should fix appveyor build for mingw + small cleanups in examples CMake configuration
There was a problem hiding this comment.
This looks good, if and when CI says it's okay I'll merge it at the earliest opportunity. Thanks for all your hard work and effort.
This is follow up on PR #140 I've just closed.
FYI: you could have forced pushed to the branch that that PR was based on and achieved the same.
This one is much better and cleaner thanks to @muggenhor 👍
I'm glad to have been able to help. Your hard work and perseverance got most of this done.
| add_library(Calc src/Calculator) | ||
| target_include_directories(Calc PUBLIC src) |
There was a problem hiding this comment.
For future reference: I would have preferred for this cleanup to happen in a separate PR, or at least a separate commit. Generally I prefer every change in one commit to work towards accomplishing one goal (which would be fixing the build for this one).
| @@ -27,6 +29,8 @@ | |||
| * Fixed `defs.hpp` deprecation warning on MSVC ([#124](https://github.com/cucumber/cucumber-cpp/pull/124) Antoine Allard) | |||
| * Fixed parallel build ([#135](https://github.com/cucumber/cucumber-cpp/pull/135) Giel van Schijndel) | |||
| * Fixed memory leaks and better memory management ([#134](https://github.com/cucumber/cucumber-cpp/pull/134) Giel van Schijndel) | |||
| * Got rid of clang warning - fix for issue #119 ([#138](https://github.com/cucumber/cucumber-cpp/pull/138) Kamil Strzempowicz, [f933ad1](https://github.com/paoloambrosio/cucumber-cpp/commit/f933ad1983cf15cc0573532f98b5457bc1ba2a18) Paolo Ambrosio) | |||
There was a problem hiding this comment.
Oh, seems @paoloambrosio forgot to add this, thanks for catching it. In general we don't attribute the maintainer that merged it nor the merge commit though (but leave it as is for now, I'll fix it up in the merge commit.
* Make building with Qt possible to disable (CUKE_DISABLE_QT) * Enable building with Qt on our CI systems for improved coverage * Refactor the CMake build system around this to be more clean * Refactor the CI configurations to be cleaner & leaner
|
Hi @konserw, Thanks for your making your first contribution to Cucumber, and welcome to the Cucumber committers team! You can now push directly to this repo and all other repos under the cucumber organization! 🍾 In return for this generous offer we hope you will:
On behalf of the Cucumber core team, |
This is follow up on PR #140 I've just closed. This one is much better and cleaner thanks to @muggenhor 👍 Ad meritum:
Main purpose of this PR is to setup Qt framework in CI on all 3 platforms:
In addition it resulted in refactoring some CMake and CI configuration files: